Skip to content

Internal enable fix #81

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
MarkRivers wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Internal enable fix #81

MarkRivers wants to merge 5 commits into from

Conversation

@MarkRivers
Copy link
Member

@MarkRivers MarkRivers commented Dec 28, 2025

Fixed issues with Internal Enable trigger mode.

  • This mode was broken completely starting with R3-4 in June 2022. It was always sending 0 as the TriggerExposure value due to a bug introduced in the driver.
  • The TriggerExposure value was only used in Internal Enable mode. This is confusing, since AcquireTime is used for all other modes.
  • The TriggerExposure record was eliminated, and AcquireTime is now used in Internal Enable mode.
    This breaks backwards compatibility, but since Internal Enable mode has not worked at all for 3.5 years, it is likely to have a small impact.
    eigerBase.template, the OPI screens, and the Sphinx documentation have been modified to remove this record.

@MarkRivers MarkRivers mentioned this pull request Dec 28, 2025
Open
@ericonr
Copy link
Member

ericonr commented Jan 8, 2026

What do you think about rebasing all commits into a single one? Given that this PR is essentially a single change, it would be helpful when reviewing the project history to have it in a single place, without small subsequent fixes.

I think it would also be nice to include the motivation in the issue and PR as part of the commit message. Something like:

 Remove TriggerExposure record, use AcquireTime in InternalEnable mode

- This mode was broken completely starting with R3-4 in June 2022. It
  was always sending 0 as the TriggerExposure value due to a bug
  introduced in the driver.
- The TriggerExposure value was only used in Internal Enable mode. This
  is confusing, since AcquireTime is used for all other modes.
- The TriggerExposure record was eliminated, and AcquireTime is now used
  in Internal Enable mode.
- This breaks backwards compatibility, but since Internal Enable mode
  has not worked at all for 3.5 years, it is likely to have a small
  impact.

eigerBase.template, the OPI screens, and the Sphinx documentation have
been modified to remove this record.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@brunoseivam brunoseivam brunoseivam approved these changes

@domitto domitto domitto approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@MarkRivers @ericonr @brunoseivam @domitto